CI:

• Linux
• Linux-aarch64
• Windows

It looks like this breaks something in test_ros2trace, which relies on test_tracetools. I see the same failure in GitHub CI: https://github.com/ros2/ros2_tracing/actions/runs/6998187248/job/19035964490?pr=83#step:4:28701. I'll try to take a look today.

It looks like this breaks something in test_ros2trace, which relies on test_tracetools. I see the same failure in GitHub CI: https://github.com/ros2/ros2_tracing/actions/runs/6998187248/job/19035964490?pr=83#step:4:28701. I'll try to take a look today.

Yeah, agreed. That's why I changed it to a draft for now. Looking at the changes, I'm surprised; none of this really should make that difference. Thanks for taking a look!

mark all dependencies as test_depend (since this is a test-only package).

test_ros2trace uses node executables from test_tracetools, so test_ros2trace depended on test_tracetools <[exec_]depend>ing on std_msgs, rclcpp, and friends: https://github.com/ros2/ros2_tracing/actions/runs/6998187248/job/19035964490?pr=83#step:4:28395

Adding <test_depend>s for std_msgs (explicitly used by the test_tracetools nodes used in the test) and rclcpp (for other stuff like statistics_msgs) to test_ros2trace's package.xml fixes the issue, but I don't think test_ros2trace should have to think about this. I would suggest moving all dependencies related to the test_tracetools nodes back to <build_depend>/<exec_depend>.

I admit that it's a bit weird because the test_tracetools node executables are only built if -DBUILD_TESTING=ON. However, if we're not building/running test_tracetools tests, then we're probably not running test_ros2trace tests either, so it makes sense not to build the nodes.

test_ros2trace uses node executables from test_tracetools, so test_ros2trace depended on test_tracetools <[exec_]depend>ing on std_msgs, rclcpp, and friends: https://github.com/ros2/ros2_tracing/actions/runs/6998187248/job/19035964490?pr=83#step:4:28395

Thanks for this analysis, it was helpful.

While looking at this, it occurred to me that there was another solution here. And that is to export the dependencies that we need from test_tracetools so that colcon will properly setup the environment. That way we can keep all of the dependencies as test_depend, but just export the ones that are needed externally. I've done that in 819c592, and it seems to work in my testing.

Thoughts on this approach?

Sorry for the delay. That's a good idea.

However, if we want test_tracetools test node executables to be used by tests in other test_* packages, then we should do this for std_srvs and rclcpp_lifecycle too (I think those are the other required dependencies if a downstream package were to use all nodes). Explicitly mentioning test_ros2trace in test_tracetools and limiting the <build_export_depend> to just those dependencies feels a bit weird (i.e., test_tracetools ideally shouldn't know about downstream packages), but I don't really mind. We can always change it in the future; it's just a test package.

However, if we want test_tracetools test node executables to be used by tests in other test_* packages, then we should do this for std_srvs and rclcpp_lifecycle too (I think those are the other required dependencies if a downstream package were to use all nodes). Explicitly mentioning test_ros2trace in test_tracetools and limiting the <build_export_depend> to just those dependencies feels a bit weird (i.e., test_tracetools ideally shouldn't know about downstream packages), but I don't really mind. We can always change it in the future; it's just a test package.

Good points. I've updated it now to build_export_depend all of the necessary dependencies, and I've also updated the language to be less specific to test_ros2trace. I'm going to pull this out of draft status now and run CI on it, thanks for the review!

CI:

• Linux
• Linux-aarch64
• Windows